Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spacing and layout of registered TopBar items #4991

Merged
merged 1 commit into from
Mar 15, 2022
Merged

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented Mar 10, 2022

Signed-off-by: Sebastian Malton [email protected]

Before:
Screen Shot 2022-03-10 at 2 54 48 PM

After (with window buttons):
Screen Shot 2022-03-10 at 2 51 10 PM

After (without window buttons):
Screen Shot 2022-03-10 at 2 51 37 PM

@Nokel81 Nokel81 added bug Something isn't working area/ui labels Mar 10, 2022
@Nokel81 Nokel81 added this to the 5.4.2 milestone Mar 10, 2022
@Nokel81 Nokel81 requested a review from a team as a code owner March 10, 2022 19:57
@Nokel81 Nokel81 requested review from jakolehm and Iku-turso and removed request for a team March 10, 2022 19:57
onClick={goForward}
disabled={!nextEnabled.get()}
/>
</div>
<div className={styles.controls}>
<div className={styles.items}>
{renderRegisteredItems(items.get())}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, it should now be relatively easy to add the ability to add items to either side via the extension API. That, however, is left for future work.

@stefcameron
Copy link
Contributor

Thank you for the fix!

Copy link
Contributor

@aleksfront aleksfront left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stefcameron stefcameron mentioned this pull request Mar 14, 2022
Copy link
Contributor

@jim-docker jim-docker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to have a default hover (hovering over the middle icon):

Screen Shot 2022-03-14 at 3 12 46 PM

@Nokel81
Copy link
Collaborator Author

Nokel81 commented Mar 14, 2022

Would you mind providing the extension registration code?

@jim-docker
Copy link
Contributor

Would you mind providing the extension registration code?

ah, never mind, just realized it is coming from my extension, sorry

Copy link
Contributor

@jim-docker jim-docker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's pretty much designer beware:

Screen Shot 2022-03-14 at 3 41 26 PM

I guess we don't clip to the top bar so we can purposefully let stuff bleed outside

@Nokel81
Copy link
Collaborator Author

Nokel81 commented Mar 14, 2022

Yeah we don't clip it at all.

Can you merge this PR once the release has been made?

@jim-docker
Copy link
Contributor

Yeah we don't clip it at all.

Can you merge this PR once the release has been made?

do we need to wait?

@Nokel81
Copy link
Collaborator Author

Nokel81 commented Mar 15, 2022

No, I realized that that release is from a different branch.

@Nokel81 Nokel81 modified the milestones: 5.4.2, 5.4.3 Mar 15, 2022
@Nokel81 Nokel81 merged commit 4d1c690 into master Mar 15, 2022
@Nokel81 Nokel81 deleted the fix-top-bar-items branch March 15, 2022 12:36
@jim-docker jim-docker mentioned this pull request Mar 17, 2022
jim-docker added a commit that referenced this pull request Mar 17, 2022
* Fix spacing and layout of registered TopBar items (#4991)

* Fix: overlapped pod logs (#5029)

* release v5.4.3

Signed-off-by: Jim Ehrismann <[email protected]>

Co-authored-by: Sebastian Malton <[email protected]>
Co-authored-by: Alex Andreev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants